Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/predictable server ids #1361

Merged

Conversation

acelaya
Copy link
Member

@acelaya acelaya commented Nov 1, 2024

Closes #1360

Change logic while creating servers, so that the server ID is computed from the name+domain, instead of generating a random UUID.

For example, for { "name": "My cool server", domain: "https://example.com", ... }, the generated ID is my-cool-server-example.com.

If the ID is duplicated, we will append an auto-increment counter, starting with 1, until no ID collission happens.

This ensures server URLs are predictable across environments, as long as names and domains are preserved. This is the case for environments where a servers.json file or env vars for a default server are provided.

As a nice side effect, the new URLs are more human friendly, and we can finally get rid of the uuid dependency without introducing breaking changes.

Existing servers that were previously created with a UUID will continue working normally, as once the ID is generated, there's no special treatment applied to it and it is simply used as is.

@acelaya
Copy link
Member Author

acelaya commented Nov 1, 2024

As opposed to previous similar attempts to achieve this, this PR:

  1. Does not leak the API key into the URL, even in the form of a hash.
  2. Generates human-friendly URLs, instead of URLs containing a hash.
  3. The URL should not be super long, unless the name/domain are in fact quite long.

@acelaya acelaya force-pushed the feature/predictable-server-ids branch from e8b9a3e to e786f9d Compare November 1, 2024 10:52
Copy link

github-actions bot commented Nov 1, 2024

Copy link

codecov bot commented Nov 1, 2024

Codecov Report

Attention: Patch coverage is 77.38095% with 19 lines in your changes missing coverage. Please review.

Project coverage is 79.66%. Comparing base (2bbc3e6) to head (b31949b).
Report is 12 commits behind head on develop.

Files with missing lines Patch % Lines
src/servers/helpers/index.ts 74.46% 0 Missing and 12 partials ⚠️
src/servers/helpers/ImportServersBtn.tsx 87.50% 0 Missing and 3 partials ⚠️
src/servers/reducers/remoteServers.ts 60.00% 0 Missing and 2 partials ⚠️
src/servers/CreateServer.tsx 83.33% 0 Missing and 1 partial ⚠️
src/servers/reducers/servers.ts 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1361      +/-   ##
===========================================
+ Coverage    79.11%   79.66%   +0.54%     
===========================================
  Files           49       49              
  Lines         1264     1303      +39     
  Branches       242      245       +3     
===========================================
+ Hits          1000     1038      +38     
+ Misses          23       21       -2     
- Partials       241      244       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@acelaya acelaya force-pushed the feature/predictable-server-ids branch from 5e95a7a to b31949b Compare November 1, 2024 11:49
@acelaya acelaya marked this pull request as ready for review November 1, 2024 11:49
@acelaya acelaya merged commit 238cefd into shlinkio:develop Nov 1, 2024
5 of 6 checks passed
@acelaya acelaya deleted the feature/predictable-server-ids branch November 1, 2024 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to deep-link when the client is shared
1 participant